Skip to content

Conversation

@rjernst
Copy link
Member

@rjernst rjernst commented May 30, 2025

Entitlement instrumentation works by reflectively calling back into the entitlements lib to grab the checker. It must be fully in place before any classes are instrumented. This commit fixes a bug that was introduced by refactoring which caused the checker to not be set until after all classes were instrumented. In some situations this could lead the checker to being null when it is grabbed (and statically cached) by the entitlement bridge.

Entitlement instrumentation works by reflectively calling back into the
entitlements lib to grab the checker. It must be fully in place before
any classes are instrumented. This commit fixes a bug that was
introduced by refactoring which caused the checker to not be set until
after all classes were instrumented. In some situations this could lead
the checker to being null when it is grab (and statically cached) by the
entitlement bridge.
@rjernst rjernst requested a review from a team as a code owner May 30, 2025 21:33
@rjernst rjernst added >non-issue auto-backport Automatically create backport pull requests when merged v8.19.0 v9.1.0 :Core/Infra/Entitlements Entitlements infrastructure labels May 30, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label May 30, 2025
Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

- class: org.elasticsearch.xpack.esql.expression.function.scalar.string.WildcardLikeTests
method: testEvaluateInManyThreads {TestCase=100 random code points matches self case insensitive with text}
issue: https://github.com/elastic/elasticsearch/issues/128677
- class: org.elasticsearch.test.apmintegration.MetricsApmIT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also TraceApmIT that needs to be unmuted.

checker = initChecker(inst, createPolicyManager());
// the checker _MUST_ be set before _any_ instrumentation is done
checker = initChecker(createPolicyManager());
initInstrumentation(inst);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that is subtle. I had to look at it for a while.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way you've written this is much more clear and robust: initialize the checker, then fire up the instrumentation.

@rjernst rjernst enabled auto-merge (squash) May 30, 2025 22:02
@rjernst rjernst merged commit 2be74a4 into elastic:main May 30, 2025
17 of 18 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.19 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 128703

rjernst added a commit to rjernst/elasticsearch that referenced this pull request May 31, 2025
Entitlement instrumentation works by reflectively calling back into the
entitlements lib to grab the checker. It must be fully in place before
any classes are instrumented. This commit fixes a bug that was
introduced by refactoring which caused the checker to not be set until
after all classes were instrumented. In some situations this could lead
the checker to being null when it is grab (and statically cached) by the
entitlement bridge.
elasticsearchmachine pushed a commit that referenced this pull request May 31, 2025
Entitlement instrumentation works by reflectively calling back into the
entitlements lib to grab the checker. It must be fully in place before
any classes are instrumented. This commit fixes a bug that was
introduced by refactoring which caused the checker to not be set until
after all classes were instrumented. In some situations this could lead
the checker to being null when it is grab (and statically cached) by the
entitlement bridge.
mridula-s109 pushed a commit that referenced this pull request Jun 2, 2025
Entitlement instrumentation works by reflectively calling back into the
entitlements lib to grab the checker. It must be fully in place before
any classes are instrumented. This commit fixes a bug that was
introduced by refactoring which caused the checker to not be set until
after all classes were instrumented. In some situations this could lead
the checker to being null when it is grab (and statically cached) by the
entitlement bridge.
mridula-s109 pushed a commit to mridula-s109/elasticsearch that referenced this pull request Jun 3, 2025
Entitlement instrumentation works by reflectively calling back into the
entitlements lib to grab the checker. It must be fully in place before
any classes are instrumented. This commit fixes a bug that was
introduced by refactoring which caused the checker to not be set until
after all classes were instrumented. In some situations this could lead
the checker to being null when it is grab (and statically cached) by the
entitlement bridge.
joshua-adams-1 pushed a commit to joshua-adams-1/elasticsearch that referenced this pull request Jun 3, 2025
Entitlement instrumentation works by reflectively calling back into the
entitlements lib to grab the checker. It must be fully in place before
any classes are instrumented. This commit fixes a bug that was
introduced by refactoring which caused the checker to not be set until
after all classes were instrumented. In some situations this could lead
the checker to being null when it is grab (and statically cached) by the
entitlement bridge.
Samiul-TheSoccerFan pushed a commit to Samiul-TheSoccerFan/elasticsearch that referenced this pull request Jun 5, 2025
Entitlement instrumentation works by reflectively calling back into the
entitlements lib to grab the checker. It must be fully in place before
any classes are instrumented. This commit fixes a bug that was
introduced by refactoring which caused the checker to not be set until
after all classes were instrumented. In some situations this could lead
the checker to being null when it is grab (and statically cached) by the
entitlement bridge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Core/Infra/Entitlements Entitlements infrastructure >non-issue Team:Core/Infra Meta label for core/infra team v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants